Conversation
…templatize props, fix psm1 and OperationTaskBase Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
src/EFCore.Tasks/buildTransitive/Microsoft.EntityFrameworkCore.Tasks.props
Show resolved
Hide resolved
… of templating Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
…, psm1, dotnet-ef Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR removes hardcoded Target Framework Monikers (TFMs) from shipped .props files and tool/task path discovery, so Arcade TFM bumps don’t require source/package layout updates across EF Core tooling.
Changes:
- Standardizes NuGet package layouts to use stable
net/andnetframework/directory prefixes (instead of TFM-versioned directories). - Retargets
EFCore.AbstractionsandEFCore.Tasksto$(NetMinimum)for consistency with other tooling projects. - Simplifies runtime path resolution in
dotnet-ef,EFCore.Tasks, and PMC tooling to loadef.dllfrom stablenet/paths.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/dotnet-ef/dotnet-ef.csproj | Packs ef.dll under a stable tools/.../tools/net folder inside the dotnet tool layout. |
| src/dotnet-ef/RootCommand.cs | Loads ef.dll from tools/net/ef.dll instead of a hardcoded TFM directory. |
| src/EFCore.Tools/tools/EntityFrameworkCore.psm1 | Updates PMC tooling to execute ef.dll from tools\net\ef.dll. |
| src/EFCore.Tools/EFCore.Tools.csproj | Packs ef.dll into tools/net/ to match the updated PMC script lookup. |
| src/EFCore.Tasks/buildTransitive/Microsoft.EntityFrameworkCore.Tasks.props | Uses stable net/netframework folder names when locating task assemblies. |
| src/EFCore.Tasks/Tasks/Internal/OperationTaskBase.cs | Simplifies ef.dll resolution to tools/net/ef.dll based on package root. |
| src/EFCore.Tasks/EFCore.Tasks.csproj | Retargets to $(NetMinimum) and updates packing to stable tasks/net, tasks/netframework, and tools/net. |
| src/EFCore.Abstractions/EFCore.Abstractions.csproj | Retargets to $(NetMinimum) to avoid hardcoding default framework properties. |
You can also share your feedback on Copilot code review. Take the survey.
| <CodeAnalysisRuleSet>$(MSBuildThisFileDirectory)..\..\rulesets\EFCore.noxmldocs.ruleset</CodeAnalysisRuleSet> | ||
| <TargetsForTfmSpecificContentInPackage>$(TargetsForTfmSpecificContentInPackage);AddPackContent</TargetsForTfmSpecificContentInPackage> | ||
| <BuildOutputTargetFolder>tasks</BuildOutputTargetFolder> | ||
| <IncludeBuildOutput>false</IncludeBuildOutput> |
There was a problem hiding this comment.
I understand the reason for this change but this will cause the pdb to be omitted from the symbols package. We should chat with NuGet on how to correctly include the symbols in that scenario.
There was a problem hiding this comment.
@zivkan are you the right contact for NuGet Pack questions? We are using TfmSpecificDebugSymbolsFile in L86 to include a pdb into the symbols package. That works well but now this PR changes the IncludeBuildOutput flag to false which makes the item not getting respected anymore (see link above). Can you please share guidance on how to do this "the right way"?
There was a problem hiding this comment.
Unfortunately, Pack hasn't had much investment since I joined the team, so I've never had the opportunity to learn the breadth of features or common problems and solution. I'm also confident that the only "friendly" helper is pack as tool for .NET tools, but we need to make improvements for packing msbuild task packages, as well as roslyn analyzers. It's a gap.
Down on line 76, I see a pdb is being added via TfmSpecificPackageFile. I would have hoped that NuGet's existing logic to split the symbols package would make this work automatically. Can we check the CI build artifacts? If it doesn't work, there's another target that includes TfmSpecificDebugSymbolsFile: https://github.com/NuGet/NuGet.Client/blob/8deb76a20811f7b78531b66a1024cfe9e650cc69/src/NuGet.Core/NuGet.Build.Tasks/NuGet.Build.Tasks.Pack.targets#L471
Hardcoded TFMs in shipping
.propsfiles and source code require manual updates whenever Arcade bumpsTargetFrameworkDefaultproperties. This eliminates all such hardcodes.Retarget to
$(NetMinimum):EFCore.Abstractions(was$(DefaultNetCoreTargetFramework)) andEFCore.Tasks(was$(NetCurrent)) now target$(NetMinimum), consistent withef,dotnet-ef, andEFCore.Tools.Stable
net/netframeworkdirectory names everywhere: All NuGet package layouts now usenetandnetframeworkas stable directory prefixes that never need updating, replacing TFM-versioned paths (e.g.net10.0). This applies to:Microsoft.EntityFrameworkCore.Tasks.props— usesnetandnetframeworkas_TaskTargetFrameworkvalues (plain.propsfile, no templating needed).EFCore.Tasks.csproj— packs totasks/net/,tasks/netframework/,tools/net/withIncludeBuildOutput=false.EFCore.Tools.csproj— packs ef.dll totools/net/instead oftools/$(TargetFramework)/any/.dotnet-ef.csproj— packs ef.dll totools/$(TargetFramework)/any/tools/net/instead oftools/$(TargetFramework)/any/tools/$(TargetFramework)/any/.Simplified
OperationTaskBase.cs: Replaced the#if NET472 / #elifguard and dynamic directory discovery with a simple hardcoded"net"path segment.Simplified
EntityFrameworkCore.psm1: Replaced dynamicGet-ChildItemdirectory discovery withJoin-Path $PSScriptRoot 'net\ef.dll'.Simplified
dotnet-ef/RootCommand.cs: Replaced the#if !NET10_0 / #errorguard and hardcoded"net10.0"path withPath.Combine(toolsPath, "net", "ef.dll").Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.